-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add system-info callback #15
Add system-info callback #15
Conversation
2745982
to
e770fb0
Compare
OS_NAME=${NAME:-""} | ||
OS_VERSION=${VERSION_ID:-""} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just thinking about if we could set some meaningful default values for $NAME
and $VERSION_ID
maybe during build as ENV
in the Dockerfile. Then a user could still overwrite these in their podTemplateSpec in the provdider config if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of this stuff is strictly informational. It's nice to have, but not strictly necessary in terms of functionality. Simply put, it has no baring on functionality, but if a user does a:
garm-cli runner show <runner ID>
that command will show the real OS and version, not something we infer from the image itself.
This was always the way I hoped this information would be sent upstream (runners would send that info as part of their installation process), but only now got around to doing it.
The idea is that by default, it should be fine to have an empty string for this stuff. But if we can safely determine the OS and version , we should send that to garm.
The /etc/os-release
file has been available for a very long time. So I expect it to be present in any container image that is based of any OS that;s not scratch
.
Thanks for the contribution @gabriel-samfira! Would it make sense to add this callback to the summerwind image as well and if so could you add it there too? :) |
not sure. Will look at that image as well. We sould theoretically do it there as well before we delegate the rest of the entrypoint execution to the default one. Will have a look later this week (traveling ATM) |
A new callback is available in garm which allows runners to send info about themselves from userdata. This new callback allows instances to report their os_name, os_version and the GH agent_id. See cloudbase/garm#200 This change allso makes sure that curl is installed in the image, otherwise the userdata will never work. Signed-off-by: Gabriel Adrian Samfira <[email protected]>
e770fb0
to
36c6ea1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also verified it locally and it looks good to me! Thank you very much and also sorry for my late response on this topic.
A new callback is available in garm which allows runners to send info about themselves while installing. This new callback allows instances to report their os_name, os_version and the GH agent_id.
See cloudbase/garm#200
This change also makes sure that curl is installed in the image, otherwise the entrypoint will fail.